Recorder exit code policy #66
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
design-docs/adr/0017-recorder-exit-code-policy.md: design-docs/recorder-exit-code-policy-implementation-plan.md: Signed-off-by: Tzanko Matev <[email protected]>
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Coverage SummaryRust (lines)
Python (statements)
Generated automatically via |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
ct recordinvokes the Rust-backedcodetracer_python_recorderCLI when capturing Python traces. The CLI currently returns the traced script's process exit code (codetracer_python_recorder/cli.py:165). When the target program exits with a non-zero status—whether viaSystemExit, a failed assertion, or an explicitsys.exit()—the recorder propagates that status. The desktop CLI treats any non-zero exit as a fatal recording failure, so trace uploads and follow-on automation abort even though the trace artefacts are valid and the recorder itself completed successfully.Our recorder already captures the script's exit status in session metadata (
runtime/tracer/lifecycle.rs:143) and exposes it through trace viewers. Downstream consumers that need to assert on the original program outcome can read that field. However, other integrations (CI pipelines,ct recordautomations, scripted data collection) rely on the CLI process exit code to decide whether to continue, and they expect Codetracer to return0when recording succeeded.We must let callers control whether the recorder propagates the script's exit status or reports recorder success independently. The default should favour Codetracer success (exit
0) to preservect recordexpectations, while still allowing advanced users and direct CLI invocations to opt back into passthrough semantics.Decision
Introduce a recorder exit-code policy with the following behaviour:
Default: When tracing completes without recorder errors (start, flush, stop, and write phases succeed and
require_tracedid not trigger), the CLI exits with status0regardless of the traced script's exit code. The recorder still records the script's status in trace metadata.Opt-in passthrough: Expose a CLI flag
--propagate-script-exitand environment overrideCODETRACER_PROPAGATE_SCRIPT_EXIT. When enabled, the CLI mirrors the traced script's exit code (the current behaviour). Both configuration surfaces resolve through the recorder policy layer so other entry points (e.g., embedded integrations) can opt in.User feedback: If passthrough is disabled and the script exits non-zero, emit a one-line warning on stderr indicating the script's exit status and how to re-enable propagation.
Recorder failure precedence: Recorder failures (startup errors, policy violations such as
--require-trace, flush/stop exceptions) continue to exit non-zero irrespective of the propagation setting to ensure automation can detect recorder malfunction.This policy applies uniformly to
python -m codetracer_python_recorder,ct record, and any embedding that drives the same CLI module.